From: Ian Jackson Date: Mon, 9 May 2011 14:00:57 +0000 (+0100) Subject: libxc: [CVE-2011-1583] pv kernel image validation X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https:/%22bookmarks://%22/%22http:/www.example.com/cgi/%22https:/%22bookmarks:/%22?a=commitdiff_plain;h=778d29bc95d42445a0ebcca1dda7adf6f95dba6f;p=xen.git libxc: [CVE-2011-1583] pv kernel image validation The functions which interpret the kernel image supplied for a paravirtualised guest, and decompress it into memory when booting the domain, are incautious. Specifically: (i) Integer overflow in the decompression loop memory allocator might result in overrunning the buffer used for the decompressed image; (ii) Integer overflows and lack of checking of certain length fields can result in the loader reading its own address space beyond the size of the supplied kernel image file. (iii) Lack of error checking in the decompression loop can lead to an infinite loop. This patch fixes these problems. CVE-2011-1583. Signed-off-by: Ian Campbell Signed-off-by: Ian Jackson Committed-by: Ian Jackson --- diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c index 9a755eb62a..4954768a0d 100644 --- a/tools/libxc/xc_dom_bzimageloader.c +++ b/tools/libxc/xc_dom_bzimageloader.c @@ -82,8 +82,29 @@ static int xc_try_bzip2_decode( for ( ; ; ) { ret = BZ2_bzDecompress(&stream); - if ( (stream.avail_out == 0) || (ret != BZ_OK) ) + if ( ret == BZ_STREAM_END ) { + DOMPRINTF("BZIP2: Saw data stream end"); + retval = 0; + break; + } + if ( ret != BZ_OK ) + { + DOMPRINTF("BZIP2: error %d", ret); + free(out_buf); + goto bzip2_cleanup; + } + + if ( stream.avail_out == 0 ) + { + /* Protect against output buffer overflow */ + if ( outsize > INT_MAX / 2 ) + { + DOMPRINTF("BZIP2: output buffer overflow"); + free(out_buf); + goto bzip2_cleanup; + } + tmp_buf = realloc(out_buf, outsize * 2); if ( tmp_buf == NULL ) { @@ -97,16 +118,18 @@ static int xc_try_bzip2_decode( stream.avail_out = (outsize * 2) - outsize; outsize *= 2; } - - if ( ret != BZ_OK ) + else if ( stream.avail_in == 0 ) { - if ( ret == BZ_STREAM_END ) - { - DOMPRINTF("BZIP2: Saw data stream end"); - retval = 0; - break; - } - DOMPRINTF("BZIP2: error"); + /* + * If there is output buffer available then this indicates + * that BZ2_bzDecompress would like more input data to be + * provided. However our complete input buffer is in + * memory and provided upfront so if avail_in is zero this + * actually indicates a truncated input. + */ + DOMPRINTF("BZIP2: not enough input"); + free(out_buf); + goto bzip2_cleanup; } } @@ -173,31 +196,14 @@ static int _xc_try_lzma_decode( for ( ; ; ) { ret = lzma_code(stream, action); - if ( (stream->avail_out == 0) || (ret != LZMA_OK) ) + if ( ret == LZMA_STREAM_END ) { - tmp_buf = realloc(out_buf, outsize * 2); - if ( tmp_buf == NULL ) - { - DOMPRINTF("%s: Failed to realloc memory", what); - free(out_buf); - goto lzma_cleanup; - } - out_buf = tmp_buf; - - stream->next_out = out_buf + outsize; - stream->avail_out = (outsize * 2) - outsize; - outsize *= 2; + DOMPRINTF("%s: Saw data stream end", what); + retval = 0; + break; } - if ( ret != LZMA_OK ) { - if ( ret == LZMA_STREAM_END ) - { - DOMPRINTF("%s: Saw data stream end", what); - retval = 0; - break; - } - switch ( ret ) { case LZMA_MEM_ERROR: @@ -231,7 +237,32 @@ static int _xc_try_lzma_decode( } DOMPRINTF("%s: %s decompression error: %s", __FUNCTION__, what, msg); - break; + free(out_buf); + goto lzma_cleanup; + } + + if ( stream->avail_out == 0 ) + { + /* Protect against output buffer overflow */ + if ( outsize > INT_MAX / 2 ) + { + DOMPRINTF("%s: output buffer overflow", what); + free(out_buf); + goto lzma_cleanup; + } + + tmp_buf = realloc(out_buf, outsize * 2); + if ( tmp_buf == NULL ) + { + DOMPRINTF("%s: Failed to realloc memory", what); + free(out_buf); + goto lzma_cleanup; + } + out_buf = tmp_buf; + + stream->next_out = out_buf + outsize; + stream->avail_out = (outsize * 2) - outsize; + outsize *= 2; } } @@ -521,18 +552,18 @@ struct setup_header { extern struct xc_dom_loader elf_loader; -static unsigned int payload_offset(struct setup_header *hdr) +static int check_magic(struct xc_dom_image *dom, const void *magic, size_t len) { - unsigned int off; + if (len > dom->kernel_size) + return 0; - off = (hdr->setup_sects + 1) * 512; - off += hdr->payload_offset; - return off; + return (memcmp(dom->kernel_blob, magic, len) == 0); } static int xc_dom_probe_bzimage_kernel(struct xc_dom_image *dom) { struct setup_header *hdr; + uint64_t payload_offset, payload_length; int ret; if ( dom->kernel_blob == NULL ) @@ -565,10 +596,30 @@ static int xc_dom_probe_bzimage_kernel(struct xc_dom_image *dom) return -EINVAL; } - dom->kernel_blob = dom->kernel_blob + payload_offset(hdr); - dom->kernel_size = hdr->payload_length; - if ( memcmp(dom->kernel_blob, "\037\213", 2) == 0 ) + /* upcast to 64 bits to avoid overflow */ + /* setup_sects is u8 and so cannot overflow */ + payload_offset = (hdr->setup_sects + 1) * 512; + payload_offset += hdr->payload_offset; + payload_length = hdr->payload_length; + + if ( payload_offset >= dom->kernel_size ) + { + xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: payload offset overflow", + __FUNCTION__); + return -EINVAL; + } + if ( (payload_offset + payload_length) > dom->kernel_size ) + { + xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: payload length overflow", + __FUNCTION__); + return -EINVAL; + } + + dom->kernel_blob = dom->kernel_blob + payload_offset; + dom->kernel_size = payload_length; + + if ( check_magic(dom, "\037\213", 2) ) { ret = xc_dom_try_gunzip(dom, &dom->kernel_blob, &dom->kernel_size); if ( ret == -1 ) @@ -578,7 +629,7 @@ static int xc_dom_probe_bzimage_kernel(struct xc_dom_image *dom) return -EINVAL; } } - else if ( memcmp(dom->kernel_blob, "\102\132\150", 3) == 0 ) + else if ( check_magic(dom, "\102\132\150", 3) ) { ret = xc_try_bzip2_decode(dom, &dom->kernel_blob, &dom->kernel_size); if ( ret < 0 ) @@ -589,7 +640,7 @@ static int xc_dom_probe_bzimage_kernel(struct xc_dom_image *dom) return -EINVAL; } } - else if ( memcmp(dom->kernel_blob, "\3757zXZ", 6) == 0 ) + else if ( check_magic(dom, "\3757zXZ", 6) ) { ret = xc_try_xz_decode(dom, &dom->kernel_blob, &dom->kernel_size); if ( ret < 0 ) @@ -600,7 +651,7 @@ static int xc_dom_probe_bzimage_kernel(struct xc_dom_image *dom) return -EINVAL; } } - else if ( memcmp(dom->kernel_blob, "\135\000", 2) == 0 ) + else if ( check_magic(dom, "\135\000", 2) ) { ret = xc_try_lzma_decode(dom, &dom->kernel_blob, &dom->kernel_size); if ( ret < 0 ) @@ -611,7 +662,7 @@ static int xc_dom_probe_bzimage_kernel(struct xc_dom_image *dom) return -EINVAL; } } - else if ( memcmp(dom->kernel_blob, "\x89LZO", 5) == 0 ) + else if ( check_magic(dom, "\x89LZO", 5) ) { ret = xc_try_lzo1x_decode(dom, &dom->kernel_blob, &dom->kernel_size); if ( ret < 0 )